feat: Adds TempoStack resource#2542
Conversation
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
|
Caution Review failedThe pull request is closed. WalkthroughAdds a new API group constant TEMPO_GRAFANA_COM and introduces a new NamespacedResource subclass TempoStack that sets api_group to that constant, accepts management_state, resources, storage, storage_size, template, validates required fields in to_dict(), and constructs the resource spec. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
ocp_resources/resource.py (1)
570-570: Consider adding type annotation for consistency.Most constants in
ApiGroupinclude: strtype annotation. Consider updating toTEMPO_GRAFANA_COM: str = "tempo.grafana.com"to match the majority pattern.ocp_resources/tempo_stack.py (2)
21-21: Consider using a more specific type forstorage_size.The
Anytype is too broad. Kubernetes storage sizes are typically strings (e.g., "10Gi", "1Ti"). Consider usingstr | Noneinstead.- storage_size: Any | None = None, + storage_size: str | None = None,
63-71: Minor: Remove extra blank line.Line 66 has an unnecessary blank line that breaks consistency with the other conditional blocks.
if self.resources is not None: _spec["resources"] = self.resources - if self.storage_size is not None: _spec["storageSize"] = self.storage_size if self.template is not None: _spec["template"] = self.template
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ocp_resources/resource.py(1 hunks)ocp_resources/tempo_stack.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
ocp_resources/tempo_stack.py (2)
ocp_resources/resource.py (4)
NamespacedResource(1535-1645)ApiGroup(466-577)to_dict(738-742)to_dict(1643-1645)ocp_resources/exceptions.py (1)
MissingRequiredArgumentError(5-10)
🔇 Additional comments (1)
ocp_resources/tempo_stack.py (1)
1-14: LGTM!The imports are appropriate, class structure is correct, and the
api_groupcorrectly references the newTEMPO_GRAFANA_COMconstant.
|
pre-commit.ci autofix |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
ocp_resources/tempo_stack.py (1)
51-55: The current implementation is correct per project conventions.The use of the "self." prefix in the
MissingRequiredArgumentErrorargument (e.g.,"self.management_state"and"self.storage") is the established convention in this codebase. Based on learnings, this pattern is defined in the code generator template and followed consistently across resource classes.
🧹 Nitpick comments (1)
ocp_resources/tempo_stack.py (1)
66-66: Consider removing the extra blank line for consistency.There's an extra blank line here between the
resourcesandstorage_sizeconditional blocks, but no blank line betweenstorage_sizeandtemplate. For consistency, consider removing this blank line.Apply this diff:
if self.resources is not None: _spec["resources"] = self.resources - if self.storage_size is not None: _spec["storageSize"] = self.storage_size
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ocp_resources/resource.py(1 hunks)ocp_resources/tempo_stack.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- ocp_resources/resource.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-11T16:42:29.245Z
Learnt from: servolkov
PR: RedHatQE/openshift-python-wrapper#2490
File: ocp_resources/route_advertisements.py:53-65
Timestamp: 2025-08-11T16:42:29.245Z
Learning: In the openshift-python-wrapper project, when raising MissingRequiredArgumentError, the convention is to include the "self." prefix in the argument name (e.g., `MissingRequiredArgumentError(argument="self.advertisements")`). This pattern is established in the code generator template at `class_generator/manifests/class_generator_template.j2` and followed consistently across most resource classes.
Applied to files:
ocp_resources/tempo_stack.py
🧬 Code graph analysis (1)
ocp_resources/tempo_stack.py (2)
ocp_resources/resource.py (4)
NamespacedResource(1535-1645)ApiGroup(466-577)to_dict(738-742)to_dict(1643-1645)ocp_resources/exceptions.py (1)
MissingRequiredArgumentError(5-10)
🔇 Additional comments (3)
ocp_resources/tempo_stack.py (3)
1-7: LGTM!The imports and header are appropriate for the TempoStack resource implementation.
9-14: LGTM!The class declaration and API group configuration are correct, properly referencing the
TEMPO_GRAFANA_COMconstant defined in the parent class.
16-45: LGTM!The
__init__method is well-structured with proper type hints, clear documentation, and correct parent class initialization.
|
pre-commit.ci autofix |
|
@kpunwatk please fix tox |
|
pre-commit.ci autofix |
modified: ocp_resources/resource.py new file: ocp_resources/tempo_stack.py modified: ocp_resources/resource.py
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
|
/approve |
|
/verified |
5 participants
Adds TempoStack resource to the wrapper
Short description:
More details:
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
Bug:
Summary by CodeRabbit
New Features
Chores